feat: add gateway import command#7
Conversation
Add `agentcore import gateway --arn <arn>` to import existing AWS gateways (with all targets) into a local CLI project. Also remove import from the HIDDEN_FROM_TUI list so it appears in the interactive TUI. - Add AWS SDK wrappers for gateway/target list/get APIs - Add import-gateway.ts with multi-resource CFN import support - Add resourceName schema field to preserve actual AWS gateway name during import - Register gateway in TUI ImportSelectScreen and ImportProgressScreen - Extend ARN pattern, deployed state, and CFN constants for gateway type
The ARN text input was truncating long ARNs. Use the expandable prop to wrap text across multiple lines. Also add gateway to the ARN validation pattern and resource type labels.
Remove --name (confusing local rename) and --yes (no prompts to confirm) from the gateway import command. The gateway's AWS name is used directly.
8a43c60 to
d2671e8
Compare
Coverage Report
|
Add end-to-end tests that create a real AWS gateway with an MCP server target, import it via `agentcore import gateway --arn`, and verify the resulting agentcore.json fields and deployed-state.json entries. New files: - e2e-tests/fixtures/import/setup_gateway.py: creates gateway + target - e2e-tests/fixtures/import/common.py: gateway wait helpers - e2e-tests/fixtures/import/cleanup_resources.py: gateway cleanup Constraint: Tests follow the existing import-resources.test.ts pattern Confidence: high Scope-risk: narrow
Extract roleArn from the AWS GetGateway response and map it to executionRoleArn in agentcore.json. On deploy, CDK uses iam.Role.fromRoleArn() instead of creating a new role, keeping the original permissions intact. Constraint: imported roles use mutable: false so CDK cannot modify them Rejected: always create new role | breaks permissions on re-import Confidence: high Scope-risk: narrow
jesseturner21
left a comment
There was a problem hiding this comment.
Code Review: Gateway Import Feature
Overview
This PR adds comprehensive gateway import functionality to the CLI. The implementation is well-structured and follows existing patterns. Here's my detailed analysis:
✅ Strengths
-
Comprehensive Implementation: The new
src/cli/commands/import/import-gateway.tshandles multiple target types (MCP Server, API Gateway, OpenAPI, Smithy, Lambda) with proper credential resolution and rollback on failure. -
Consistent Architecture: Follows established patterns from runtime/memory/evaluator imports with proper separation of concerns.
-
Good Test Coverage: E2E tests verify gateway creation, import, field preservation, and deployed state structure.
-
Schema Improvements: Added
resourceNameandexecutionRoleArnfields to preserve AWS resource names during import.
⚠️ Issues & Suggestions
1. Type Safety Concerns
File: src/cli/commands/import/import-gateway.ts (lines 70-100, 280-320)
Multiple eslint-disable blocks for unsafe type operations. Consider creating typed interfaces for AWS SDK responses:
// Instead of: const mcp = response.targetConfiguration.mcp as any;
interface McpTargetConfig {
mcpServer?: { endpoint: string };
apiGateway?: { restApiId: string; stage: string; /* ... */ };
// ...
}2. Fragile String Matching in Cleanup Script
File: e2e-tests/fixtures/import/cleanup_resources.py (lines 54-56)
if "gateway" in key and "target" in key:This string matching is fragile. Consider using a more explicit key format or enum-like approach to avoid false positives.
3. Nested Path Traversal Needs Defensive Coding
File: src/cli/commands/import/import-utils.ts (lines 308-312)
let collection: any = targetState.resources;
for (const key of collectionKey.split('.')) {
collection = collection?.[key];
}If targetState.resources is undefined, this silently fails. Add explicit null check:
if (!targetState.resources) return undefined;4. Hardcoded External Endpoint in Tests
File: e2e-tests/fixtures/import/setup_gateway.py (line 75)
"endpoint": "https://mcp.exa.ai/mcp",This external dependency could cause test flakiness. Consider making it configurable via environment variable.
5. Missing --name Option in Command Registration
File: src/cli/commands/import/import-gateway.ts (lines 620-625)
The command only registers --arn but the handler supports options.name. Add the missing option:
.option('--name <name>', 'Local name for the imported gateway')6. ARN Pattern Duplication
Files: src/cli/commands/import/import-utils.ts, src/cli/tui/screens/import/ArnInputScreen.tsx
The ARN regex pattern is duplicated in multiple files. Extract to a shared constant in constants.ts.
7. Gateway Status Warning Should Be More Prominent
File: src/cli/commands/import/import-gateway.ts (lines 350-352)
if (gatewayDetail.status !== 'READY') {
onProgress(`Warning: Gateway status is ${gatewayDetail.status}, not READY`);
}Consider adding a --force flag or failing by default for non-READY gateways, as importing a non-READY gateway could lead to unexpected behavior.
8. Test Assertion Could Be More Specific
File: e2e-tests/import-resources.test.ts (line 230)
expect(gw.resourceName, 'resourceName should preserve AWS name').toBeTruthy();This only checks truthiness. Consider verifying the actual value matches the expected gateway name pattern.
📋 Minor Suggestions
- Add JSDoc comments to exported functions like
handleImportGateway()andtoGatewayTargetSpec() - The
handleImportGatewayfunction is ~300 lines - consider extracting validation and mapping into separate helper functions - Add unit tests for
toGatewayTargetSpecwith various target configurations
Summary
This is a solid, production-ready implementation that extends the import functionality consistently. The main recommendations are:
- Improve type safety by reducing
anyusage - Add the missing
--nameCLI option - Make test endpoints configurable
- Add defensive null checks for nested path traversal
Verdict: ✅ Approve with minor suggestions
|
Issue: Unsafe type assertions in gateway target mapping In Options to address: Option A — Add runtime validation: Use a schema validation library (like Zod) to validate the structure of Option B — Create typed helper functions: Extract the unsafe type casting into separate, well-documented helper functions with explicit type guards that validate the structure before use. |
|
Issue: Incomplete error handling in credential resolution In Options to address: Option A — Fail fast on missing credentials: Return an error result from Option B — Require explicit credential mapping: Add a |
|
Issue: Fragile logical ID matching for CloudFormation import In Options to address: Option A — Require explicit matching: Fail the import if any target cannot be matched by name, requiring users to ensure target names in the CDK template match the AWS gateway targets exactly. Option B — Use stable identifiers: Instead of relying on name matching, use the target ID (which is stable) to match logical IDs in the template, or require the CDK template to include target IDs as properties for matching. |
|
Issue: Unvalidated nested path traversal in deployed state lookup In Options to address: Option A — Add defensive checks and logging: Validate each step of the path traversal and log warnings when intermediate keys are missing or not objects, helping users understand why resource lookup failed. Option B — Use explicit path accessors: Replace the generic dot-notation traversal with explicit, type-safe accessors for each resource type, making the code more maintainable and easier to debug. |
|
Issue: Missing validation for gateway status before import In Options to address: Option A — Fail on non-READY status: Return an error if the gateway status is not 'READY', requiring users to wait for the gateway to be fully created before importing. Option B — Add a wait-for-ready option: Implement a |
|
Issue: Incomplete rollback on config write failure In Options to address: Option A — Atomic state updates: Ensure both config and deployed state are updated together, or implement a two-phase commit pattern where the CDK pipeline is only executed after both updates succeed in a dry-run. Option B — Add deployed state rollback: Extend the rollback function to also delete the imported CloudFormation resources if the config write fails, ensuring consistency between config and deployed state. |
Add @internal exports for toGatewayTargetSpec, resolveOutboundAuth, toGatewaySpec, and buildCredentialArnMap to enable direct unit testing of the pure mapping functions in import-gateway.ts. Confidence: high Scope-risk: narrow
…lution Bugbash coverage for toGatewayTargetSpec and resolveOutboundAuth: - mcpServer with no auth, OAuth, and API_KEY credentials - Credential resolution warnings when ARNs not in project - Targets with no MCP configuration - OAuth scopes pass-through and empty scopes omission 8 tests, all passing. Confidence: high Scope-risk: narrow
…da target mapping Bugbash coverage for toGatewayTargetSpec non-mcpServer target types: - apiGateway: restApiId, stage, toolFilters, toolOverrides mapping - openApiSchema: S3 URI mapping, missing URI warning - smithyModel: S3 URI mapping, missing URI warning - lambda: S3 tool schema to lambdaFunctionArn mapping, missing ARN, inline-only schema warning, progress messages - Unrecognized target type warning 13 tests, all passing. Confidence: high Scope-risk: narrow
Bugbash coverage for toGatewaySpec AWS-to-CLI schema mapping: - Authorizer types: NONE, AWS_IAM, CUSTOM_JWT with all JWT fields - CUSTOM_JWT customClaims with full claim structure - Semantic search: SEMANTIC/KEYWORD/missing protocolConfiguration - Exception level: DEBUG/undefined/other values - Policy engine: ARN name extraction, mode preservation - Optional fields: resourceName, description, tags, executionRoleArn - Edge cases: empty tags object omitted, empty JWT arrays omitted 23 tests, all passing. Confidence: high Scope-risk: narrow
Bugbash coverage for the main gateway import flow: - Happy path: successful import with --arn, config written, result verified - Rollback: pipeline failure restores original config, noResources error - Duplicate detection: name collision, resource ID already tracked - Name validation: invalid name regex, --name override preserves resourceName - Auto-select: single gateway auto-selected, multiple gateways error, no gateways error - Target mapping: skipped targets warning, non-READY gateway continues 12 tests, all passing. Confidence: high Scope-risk: narrow
* fix: display session ID after CLI invoke completes (closes aws#664) The streaming and non-streaming invoke responses include a session ID from the runtime, but the CLI paths discarded it. Now prints the session ID and a resume command hint after invoke output. * fix: include sessionId in AGUI protocol invoke result
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
…ws#953) Closes aws#840 When invoking an agent with a bearer token (OAuth/CUSTOM_JWT) and no session ID, `AgentCoreMemoryConfig` raised a Pydantic validation error because `session_id=None` is rejected. Unlike SigV4 callers, bearer-token callers do not get a server-side auto-generated runtime session ID. Two-layer fix: 1. CLI synthesizes a UUID in `invoke` action when `--bearer-token` is set and `--session-id` is missing, using the existing `generateSessionId` helper. Covers both explicit `--bearer-token` and the CUSTOM_JWT auto-fetch path. 2. Strands memory session templates (http, agui, a2a) synthesize a UUID when `session_id` is falsy before constructing AgentCoreMemoryConfig. Protects direct runtime callers (curl, custom apps) who forget the `X-Amzn-Bedrock-AgentCore-Runtime-Session-Id` header. Snapshot tests updated.
…ws#952) The deploy TUI appeared frozen for 5-15 seconds between preflight completion and 'Publish assets' while cdkToolkitWrapper.diff() ran silently with no step marked as running. Add a dedicated pre-deploy diff step that transitions running -> success around the diff call so StepProgress always has something to highlight. Closes aws#781
…ws#974) * feat(invoke): add --prompt-file and stdin support for long prompts Long prompts hit shell argument limits (E2BIG, typically 128KB-2MB) when passed as positional args. This adds two new sources: - --prompt-file <path>: read prompt from a file - piped stdin: when no prompt is given and stdin is not a TTY, read the prompt from stdin Precedence is hybrid and backward-compatible: --prompt > positional > --prompt-file > stdin --prompt-file combined with piped stdin content returns an explicit collision error rather than silently picking one. Closes aws#686 * docs(invoke): document --prompt-file and stdin support
The import feature has stabilized and no longer needs the experimental label.
…loses aws#637) (aws#955) - Return null during brief transitional phases to prevent Ink from rendering a header that gets immediately replaced by a different frame - Consolidate CreateScreen phases into a single Screen mount - Make help menu description width responsive to terminal size - Remove hardcoded 50-char description truncation limit
…eline After `agentcore remove gateway`, the gateway entry remains in deployed-state.json (correctly, since CFN still manages it) but is removed from agentcore.json. A subsequent `agentcore import gateway` would reject with "already imported" because the dedup check only looked at deployed-state. Now, when a resource exists in deployed-state but not in agentcore.json, the import skips the CDK pipeline and just re-adds the resource to agentcore.json. Applies to both the gateway-specific and generic import orchestrators.
- B4: Hard-fail when credential provider ARN is not found in deployed state instead of silently dropping outboundAuth - B7: Preserve outboundAuth on lambda→lambdaFunctionArn mapping and allow OAUTH/NONE auth types for lambdaFunctionArn targets - H2: Remove re-import fast path; run full CDK pipeline so out-of-band targets are properly imported. Treat noResources as success for re-imports since all resources are already in the CFN stack - H5: Replace hardcoded arn:aws: with partition-agnostic arn:[^:]+: in ARN validation and region extraction regexes - H7/H8: Add regex validation and max length for executionRoleArn, use GatewayNameSchema for resourceName, add refine ensuring both fields are set together or both omitted
…eQL alert CodeQL flagged clear-text logging of credential provider ARNs. The target name provides sufficient context for the user to identify the issue.
resourceName and executionRoleArn must co-vary per the schema refine. When a gateway has no roleArn (e.g. setup_gateway.py in e2e tests), resourceName must also be omitted to pass validation.
Summary
agentcore import gateway --arn <arn>command to import existing AWS gateways (with all targets) into a local CLI projectimportfrom the TUI so it appears in the interactive command listresourceNameschema field so CDK uses the actual AWS gateway name during import (fixes CFN IMPORT name mismatch)Changes
New file:
src/cli/commands/import/import-gateway.ts— Full gateway import implementation with multi-resource CFN import support (gateway + N targets)Modified files:
src/cli/aws/agentcore-control.ts— AWS SDK wrappers for gateway/target list/get APIssrc/cli/commands/import/command.ts— Register gateway subcommandsrc/cli/commands/import/constants.ts— Add GatewayTarget CFN resource identifiersrc/cli/commands/import/import-utils.ts— Extend ARN pattern, deployed state, constants for gateway typesrc/cli/commands/import/types.ts— Add'gateway'to importable resourcessrc/cli/tui/screens/import/ImportSelectScreen.tsx— Add Gateway option to TUIsrc/cli/tui/screens/import/ImportProgressScreen.tsx— Add gateway handler dispatchsrc/cli/tui/screens/import/ArnInputScreen.tsx— Add gateway to ARN validation, use expandable inputsrc/cli/tui/utils/commands.ts— Removeimportfrom HIDDEN_FROM_TUIsrc/schema/schemas/mcp.ts— Add optionalresourceNamefield to gateway schemaTest plan
npm run buildpasses (typecheck)npm testpasses (all 3301 tests, 0 failures)agentcore import gateway --arn <real-arn>imports gateway + targets into agentcore.json and deployed-state.jsonagentcore deployafter import succeeds without CFN name mismatchimportcommand appears when searching in the interactive TUI